Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: receiver behaviour #110

Merged
merged 7 commits into from
Jun 21, 2022
Merged

refactor: receiver behaviour #110

merged 7 commits into from
Jun 21, 2022

Conversation

mabdh
Copy link
Member

@mabdh mabdh commented Jun 20, 2022 •
edited
Loading

This is rebased to this PR so it can be reviewed once the PR has been merged.

Summary

  • Add a new abstraction for receiver type services to avoid switch case and scattered logic.
    • Centralize specific receiver type logic inside a receiver type service e.g. all slack logic will be inside slack service.
  • Refactor SendNotification feature to be more generic.
  • Refactor receiver and namespace API ref
  • Fixed all acronyms to use all capitals letter

@mabdh mabdh linked an issue Jun 20, 2022 that may be closed by this pull request
Refactor receiver to generalize the implementation #104
Closed
@mabdh mabdh force-pushed the refactor-receiver branch from d021313 to 88b4b75 Compare June 20, 2022 08:11
@coveralls
Copy link

coveralls commented Jun 20, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 2532655769

  • 384 of 455 (84.4%) changed or added relevant lines in 22 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-1.2%) to 85.286%
Changes Missing Coverage Covered Lines Changed/Added Lines %
core/subscription/service.go 11 12 91.67%
pkg/slack/client.go 5 6 83.33%
core/receiver/notification.go 9 13 69.23%
core/receiver/receiver.go 8 13 61.54%
pkg/slack/message.go 30 35 85.71%
core/receiver/http_service.go 9 21 42.86%
core/receiver/pagerduty_service.go 9 21 42.86%
core/receiver/service.go 41 56 73.21%
core/receiver/slack_service.go 61 77 79.22%
Files with Coverage Reduction New Missed Lines %
core/receiver/service.go 4 78.26%
Totals Coverage Status
Change from base Build 2527294786: -1.2%
Covered Lines: 1907
Relevant Lines: 2236
💛 - Coveralls

@mabdh mabdh marked this pull request as ready for review June 20, 2022 08:19
@mabdh mabdh requested review from rahmatrhd, pyadav and ravisuhag June 20, 2022 08:19
rahmatrhd
rahmatrhd reviewed Jun 21, 2022
View reviewed changes
pkg/slack/message.go Outdated Show resolved Hide resolved
@mabdh mabdh requested a review from rahmatrhd June 21, 2022 02:36
@mabdh mabdh merged commit 0d71c71 into main Jun 21, 2022
@mabdh mabdh deleted the refactor-receiver branch June 21, 2022 03:27
ravisuhag pushed a commit that referenced this pull request Aug 21, 2022
@mabdh
refactor: receiver behaviour (#110)
c432982 
* refactor: create strategy receiver

* refactor: replace all acronyms with all capitals

* refactor(receiver): make notify receiver function more generic

* refactor(api): refactor namespace and receiver api

* fix: lint error

* refactor: rename strategy to type receiver

* refactor: some variable declaration
ravisuhag pushed a commit that referenced this pull request Aug 21, 2022
@mabdh
refactor: receiver behaviour (#110)
9a44342 
* refactor: create strategy receiver

* refactor: replace all acronyms with all capitals

* refactor(receiver): make notify receiver function more generic

* refactor(api): refactor namespace and receiver api

* fix: lint error

* refactor: rename strategy to type receiver

* refactor: some variable declaration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rahmatrhd rahmatrhd rahmatrhd approved these changes

@pyadav pyadav Awaiting requested review from pyadav

@ravisuhag ravisuhag Awaiting requested review from ravisuhag

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor receiver to generalize the implementation
3 participants
@mabdh @coveralls @rahmatrhd